Skip to content

feat: add desktop notifications v1#262

Open
whoisasx wants to merge 2 commits into
mainfrom
codex/notifications-v1
Open

feat: add desktop notifications v1#262
whoisasx wants to merge 2 commits into
mainfrom
codex/notifications-v1

Conversation

@whoisasx

Copy link
Copy Markdown
Collaborator

Problem

Notifications were partially implemented in the Go rewrite: the daemon could create durable unread notification rows and stream new rows, but there was no acknowledgement path and the Electron app had no notification center, unread catch-up, mark-read controls, or app-owned toast bridge.

Scoped V1 behavior

  • Desktop notifications are owned by the Electron app UI, not machine-level daemon notification delivery.
  • The backend notification service remains the source of truth for notification content, status, and target metadata.
  • The renderer fetches unread notifications through REST and uses the live stream only for subsequent arrivals/invalidation.
  • Users can mark one notification read or mark all unread notifications read.
  • Rows do not navigate on click; each notification has an explicit open-target action and a separate mark-read action.
  • Live SSE notifications show Electron app toasts while the app is running. Initial unread backfill does not show toasts.

Backend changes

  • Added sqlc queries and SQLite store methods for MarkNotificationRead and MarkAllNotificationsRead.
  • Extended service/notification with MarkRead and MarkAllRead; single mark-read is an unread-to-read transition and returns NOTIFICATION_NOT_FOUND for unknown or already-read IDs.
  • Added REST routes:
    • PATCH /api/v1/notifications/{id} with { "status": "read" }
    • POST /api/v1/notifications/read-all
  • Regenerated sqlc output, OpenAPI YAML, and frontend TypeScript schema.
  • Added focused store, service, controller, and API spec parity coverage.

Frontend changes

  • Added typed unread notification query/mutations backed by the generated API client.
  • Added a notification stream transport for /api/v1/notifications/stream that:
    • invalidates unread notifications on open/reconnect/base URL changes,
    • merges live notifications idempotently by id,
    • shows Electron toasts only for newly inserted live notifications.
  • Added a persistent topbar notification center with unread badge, newest-first list, empty/error states, explicit open-target buttons, per-item mark-read, and mark-all-read.
  • Added Electron preload/main bridge for app-owned native toasts and toast-click routing back to the renderer by notification id.
  • Updated docs status to reflect the shipped V1 notification surface.

Tests run

  • npm run sqlc
  • npm run api
  • cd backend && go test ./internal/service/notification ./internal/storage/sqlite/store ./internal/httpd/controllers ./internal/httpd/apispec ./internal/notify ./internal/integration
  • cd backend && go test ./...
  • npm --prefix frontend test -- src/renderer/lib/notifications.test.ts
  • npm run frontend:typecheck
  • git diff --check

Intentional omissions / future work

  • No OS-level daemon/background notifier, Slack/Discord/webhook/email, or other external notification channels.
  • No CLI notification commands.
  • No durable replay on /api/v1/notifications/stream; REST unread fetch is the catch-up path.
  • No project-scoped read-all filter in V1.
  • PR targets open the external PR URL when provided; this can move to an internal PR detail route later if one becomes stable.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@whoisasx whoisasx requested a review from Copilot June 16, 2026 19:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@neversettle17-101

Copy link
Copy Markdown
Collaborator

Please add the testing details for the end to end flow with frontend as well in the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants